-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add registry addon feature for docker on mac/windows #7603
Conversation
Hi @alonyb. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: alonyb The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Can one of the admins verify this patch? |
documentation
Update docker.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seem comments.
Travis tests have failedHey @alonyb, TravisBuddy Request Identifier: 0829ed00-7ff2-11ea-a591-33c228b1c119 |
/ok-to-test |
kvm2 Driver Times for Minikube (PR 7603): [63.09915510499999 63.879845221000004 63.826361764] Averages Time Per Log
docker Driver Times for Minikube (PR 7603): [24.222345756000003 24.267185732999998 26.296727185] Averages Time Per Log
|
kvm2 Driver Times for Minikube (PR 7603): [64.31170485599999 63.511321812 63.518253224999995] Averages Time Per Log
docker Driver Times for Minikube (PR 7603): [23.046477247 25.416571617000002 27.458543809999995] Averages Time Per Log
|
kvm2 Driver Times for Minikube (PR 7603): [64.594298549 63.675055674 62.801884367999996] Averages Time Per Log
docker Driver Times for Minikube (PR 7603): [25.375156414 26.359010874 25.531028623] Averages Time Per Log
|
kvm2 Driver Times for Minikube (PR 7603): [64.059044793 63.849663973 63.572789532] Averages Time Per Log
docker Driver Times for Minikube (PR 7603): [25.478499490999997 25.305843879999994 25.295407432999994] Averages Time Per Log
|
kvm2 Driver Times for Minikube (PR 7603): [63.560487265 61.733120754 62.82873433899999] Averages Time Per Log
docker Driver Times for Minikube (PR 7603): [25.530326429 24.818172682 25.651925609] Averages Time Per Log
|
kvm2 Driver Times for Minikube (PR 7603): [61.14687640599999 67.86599335700001 62.392238875] Averages Time Per Log
docker Driver Times for Minikube (PR 7603): [25.562342911 25.367611718000003 26.617442934000003] Averages Time Per Log
|
kvm2 Driver Times for Minikube (PR 7603): [62.732414602000006 65.782731854 65.388474019] Averages Time Per Log
docker Driver Times for Minikube (PR 7603): [24.821625769000004 25.845501355 26.251828668] Averages Time Per Log
|
pkg/addons/addons.go
Outdated
if err != nil { | ||
return errors.Wrap(err, "registry port") | ||
} | ||
out.T(out.Documentation, `Registry addon on with {{.driver}} uses {{.port}} please use that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change this to two lines
out.T(out.Tip, Registry addon on with {{.driver}} uses {{.port}} please use that instead of default 500.
)
out.T(out.Documentation, For more information see: https://minikube.sigs.k8s.io/docs/drivers/{{.driver}}
)
also make sure the link says {{.driver}} not docker
pkg/addons/addons.go
Outdated
@@ -176,6 +178,17 @@ https://github.com/kubernetes/minikube/issues/7332`, out.V{"driver_name": cc.Dri | |||
return nil | |||
} | |||
|
|||
if name == "registry" { | |||
if driver.IsKIC(cc.Driver) && runtime.GOOS != "linux" { | |||
port, err := kic.GetRegistryAddonPort(cc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6
also get rid of
func GetRegistryAddonPort(cc *config.ClusterConfig) (int, error)
insead of that just directly call the oci.ForwardedPort(cc.Driver, cc.Name, constants.RegistryAddonPort)
it is a one line thing
kvm2 Driver Times for Minikube (PR 7603): [62.323305223000006 64.509571439 63.15886923299999] Averages Time Per Log
docker Driver Times for Minikube (PR 7603): [26.237618879 26.547181863999995 25.82037073] Averages Time Per Log
|
This PR is for #7535
Only in Mac/Windows on KIC this is the output.
Output: